How NOT to write a shared library

[j_g]

I'm interested in audio, so I'm looking over the (frankly horrible) documentation for this new "Pulse Audio" thing, and I see this function in its shared lib:

pa_xmalloc()

Allocates the specified number of bytes, just like malloc() does.

However, in case of OOM, terminate.

First of all, why are so many programmers who write a Linux shared library offering a simple wrapper function around malloc? Is it too hard for an app to just call malloc? Is anyone so completely absent-minded that he can't remember malloc is used to allocate memory, so he needs an equivalent wrapper in every shared lib he uses?

Second of all, in the case of "out of memory", it just terminates your app right then and there??? Oh great, The enduser spends an hour working on something, does some operation, and BANG! -- all his hard work goes down the drain because some shared library decides it wants to terminate the app. Absolutely ridiculous. And I see that other pulse audio functions call this pa_xmalloc too, so good luck getting around it.

I can just see it now. Every programmer who uses pulse audio will need to add a feature to his app called "Auto-save before every Pulse Audio call because this may be your last chance to preserve your work". Yeah, that should really yield "low latency".

Sorry, but anyone who puts a call to exit() in a shared library should simply be shot dead. That's really, really crummy, amateurish programming. In a shared lib, if something fails, the professional thing to do is return some error code/signal to the app, indicating that the function failed. Let the app decide what it wants to do. You don't terminate the app on your own.

And anyone who makes such a shared library a part of his operating system should be shot dead, and then stabbed through the heart with a stake just to make sure he's really dead.

God help us if this pulse audio thing is ever chosen to replace ALSA in the kernel. This will make Linux stability and reliability go to hell if you have operating system calls terminating apps at will.

[SteveHillier]

I would like to echo the sentiment of this post.
In the days when I programmed we often had to fit stuff into the small model (64K code and data) and if we were lucky the next one up (64K code, 64K data).
That forced you to write efficient code - pointers not indexed arrays etc, functions not inline code etc.
Now we have almost limitless memory it has led to some poor programming.
Just to add that of course every function should really return with an termination code. One case I remember the programmer was using long jumps on error leaving all sorts of crap behind when he did so, such as not forcing data writes, not unallocating memory.
Eh! those were the days!!

[public_void]

Every call to malloc should be checked. So rather than put the checking code at every point you call malloc, you put that code in one function normally named 'xmalloc', and call that instead.

[Wybiral]

Originally Posted by public_void Every call to malloc should be checked. So rather than put the checking code at every point you call malloc, you put that code in one function normally named 'xmalloc', and call that instead.

But you still need to check for an exception in the calling code. That's the entire reason malloc returns NULL when it fails... That's your exception.

[skeeterbug]

The real kicker here is that you spent 10-20 minutes writing this post. Did you even bother to spend time emailing the author of the library to express your concerns? Probably not. Yeah, everyone should be "shot dead" because how they think something should be done. Give me a break.

[j_g]

Originally Posted by SteveHillier the programmer was using long jumps on error leaving all sorts of crap behind when he did so

Oh yeah, I forgot about the exceedingly evil longjmp(). I actually saw someone use that in recursive code. I was utterly appalled.

longjmp() is a careless approach toward error handling that only a lazy and negligent programmer would use. Code that uses longjmp is historically buggy and unstable. If someone is using longjmp, he seriously needs to rethink his design.

<snip>

[j_g]

Originally Posted by public_void Every call to malloc should be checked. So rather than put the checking code at every point you call malloc, you put that code in one function normally named 'xmalloc', and call that instead.

So you call exit() right then and there, and the hell with preservation of the enduser's hard work, or the reliability of any software that may use your shared lib with exit()'s in it?

Sorry, but I do not find this technique to be acceptable in terms of reliability, stability, nor even usability. I would never use software that could, at any moment, dump the hard work I had done, or make running apps just disappear at random in a puff of smoke.

Please rethink your error handling techniques, for the sake of endusers everywhere.

[j_g]

Originally Posted by skeeterbug You spent 10-20 minutes writing this post. Did you even bother to spend time emailing the author of the library to express your concerns?

If the pulse audio author actually had his software peer-reviewed before it was put into Fedora, and nobody had balked at this, then I have no faith whatsoever in his peer-review process. A shared lib doesn't need to terminate an app that uses it. ALSA doesn't do it, and there's lots of malloc calls there. Calling exit() in a shared lib is simply sloppy, negligent error handling. No experienced programmer capable of writing quality code should need to be told this. He should already know it.

[public_void]

So you call exit() right then and there, and the hell with preservation of the enduser's hard work, or the reliability of any software that may use your shared lib with exit()'s in it?

I never said you should call exit(). I agree you should not call exit() in a library. However you could put error handling code in xmalloc meaning you don't have to put that code throughout your program. The error handling code may be simple such as printing to stderr. Despite having to check the return value of xmalloc, the point is to not to have code repetition to handle the same error after every call.

Edit: After reading the code (xmalloc.c) the author does write to stderr.

[Wybiral]

Originally Posted by public_void I never said you should call exit(). I agree you should not call exit() in a library. However you could put error handling code in xmalloc meaning you don't have to put that code throughout your program. The error handling code may be simple such as printing to stderr. Despite having to check the return value of xmalloc, the point is to not to have code repetition to handle the same error after every call.

I think the biggest point is that malloc has error handling, it returns NULL. That's a fine exception that your code can catch. I guess it could print to stderr, but even then I think it would be better to say "unable to allocate audio track 'file.ogg'" as opposed to "allocation error". The first is only possible if the calling code catches the exception.